feat(uart): Add function to invert hardware UART Tx line #11428

asund · 2025-06-04T04:43:00Z

Description of Change

Adds a setTxInvert(), setRtsInvert() and setCtsInvert() to HardwareSerial and corresponding uartSetTxInvert to the HAL.
Using IDF Calls instead of poking registers.

Tests scenarios

I have tested my Pull Request on Arduino-esp32 core v3.2.0 with ESP32 and ESP32-S2 Board.
I added an untested implementation for the ESP32C6, ESP32H2, ESP32P4 platform in the HAL to reflect the different register name.

Tested with CI uart.ino and using a data analizer.

Related links

N/A

Simply clone existing Rx functionality for Tx. Allow granular control over both lines. Avoid overloading HardwareSerial::begin() to change the bool invert parameter to a bitmask type. Add an untested implementation for ESP32C6, ESP32H2, ESP32P4 that references the different register naming on those chips.

github-actions · 2025-06-04T04:43:35Z

	Warnings
⚠️	Some issues found for the commit messages in this PR: the commit message `"Refactor UART inversion functions to use new method"`: summary looks empty type/action looks empty the commit message `"feat(uart) : Refactor serial inversion methods to return bool"`: summary looks empty type/action looks empty the commit message `"feat(uart): Refactor UART signal inversion handling"`: body's lines must not be longer than 100 characters Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: `<type/action>(<scope/component>): <summary>`, for example `fix(esp32): Fixed startup timeout issue` allowed types are: `change,ci,docs,feat,fix,refactor,remove,revert,test` sufficiently descriptive message summary should be between 10 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) `TIP:` Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

👋 Hello asund, we appreciate your contribution to this project!

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against e6e09ce

github-actions · 2025-06-04T15:32:26Z

Test Results

76 files 76 suites 16m 18s ⏱️
38 tests 38 ✅ 0 💤 0 ❌
241 runs 241 ✅ 0 💤 0 ❌

Results for commit e6e09ce.

♻️ This comment has been updated with latest results.

SuGlider · 2025-06-05T01:51:33Z

This PR may need a few improvements based on how IDF 5.4.1 has changed UART driver.
@asund - do you mind if I commit changes over your PR?

asund · 2025-06-05T02:06:52Z

This PR may need a few improvements based on how IDF 5.4.1 has changed UART driver. @asund - do you mind if I commit changes over your PR?

No problem at all. Let me know if there's anything I can do if you want.

SuGlider · 2025-06-21T21:17:48Z

Waiting on changes from #11499 to be merged.
The code shall be changed to add IDF calls instead which shall work for all SoC with no need to test which SoC is used in order to use the right UART Structure Names.

Possible API for inverting any signal, including CTS/RTS and LP UART signals.

asund · 2025-06-22T05:47:53Z

Sounds great! No problem closing this issue when that lands.

Refactor UART inversion functions to use a helper for signal inversion. Update UART bus array structure to include inversion mask.

Added functions for UART signal inversion and updated existing function signatures.

Changed setRxInvert, setTxInvert to return bool. Added setCtsInvert and setRtsInvert methods.

Added functions for UART pins signal inversion.

cores/esp32/esp32-hal-uart.h

Added logging for signal inversion in UART functions.

cores/esp32/esp32-hal-uart.c

Copilot

Pull Request Overview

This PR enhances UART signal inversion functionality by extending support beyond RX inversion to include TX, CTS, and RTS signal inversion. The implementation introduces a generic helper function to manage signal inversion state and exposes new APIs through the HardwareSerial class.

Adds TX, CTS, and RTS signal inversion functions alongside the existing RX inversion
Introduces an inv_mask field to track combined inversion states across all UART pins
Refactors signal inversion to use a centralized helper function that properly maintains inversion state
Updates return types from void to bool to indicate operation success

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
cores/esp32/esp32-hal-uart.h	Declares new signal inversion functions and helper, updates `uartSetRxInvert` return type to `bool`
cores/esp32/esp32-hal-uart.c	Implements generic inversion helper and individual TX/CTS/RTS inversion functions, adds `inv_mask` field to uart_struct_t, updates initialization arrays
cores/esp32/HardwareSerial.h	Declares new signal inversion methods with `bool` return types
cores/esp32/HardwareSerial.cpp	Implements wrapper methods for new signal inversion functions
tests/validation/uart/uart.ino	Adds test cases for TX signal inversion in both enabled and disabled UART states

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cores/esp32/esp32-hal-uart.h

cores/esp32/esp32-hal-uart.c

cores/esp32/HardwareSerial.h

cores/esp32/esp32-hal-uart.c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

cores/esp32/esp32-hal-uart.c:1619

The uart_send_break function does not preserve the existing inversion mask when toggling the TX inversion. This will clear any previously set signal inversions (RX, CTS, RTS). The function should use the uart structure's inv_mask to preserve other signal inversions. Consider using _uart_bus_array[uartNum].inv_mask to get the current mask, toggle only the TX bit, and restore the original mask after the break.

  uart_set_line_inverse(uartNum, UART_SIGNAL_TXD_INV);
  esp_rom_delay_us(breakTime);
  uart_set_line_inverse(uartNum, UART_SIGNAL_INV_DISABLE);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

P-R-O-C-H-Y requested a review from SuGlider June 4, 2025 09:55

P-R-O-C-H-Y added Peripheral: UART Related to the UART peripheral or its functionality. Status: Review needed Issue or PR is awaiting review labels Jun 4, 2025

Merge branch 'master' into uart_tx_invert

948c485

asund requested review from a team, P-R-O-C-H-Y and lucasssvaz as code owners June 4, 2025 13:51

SuGlider self-assigned this Jun 5, 2025

asund mentioned this pull request Jun 15, 2025

Remove dual UART implementation asund/esphome-daikin-s21#11

Closed

SuGlider added this to Arduino ESP32 Core Project Roadmap Jun 21, 2025

github-project-automation bot moved this to Todo in Arduino ESP32 Core Project Roadmap Jun 21, 2025

SuGlider moved this from Todo to Under investigation in Arduino ESP32 Core Project Roadmap Jun 21, 2025

SuGlider added this to the 3.2.1 milestone Jun 21, 2025

SuGlider added Status: In Progress ⚠️ Issue is in progress and removed Status: Review needed Issue or PR is awaiting review labels Jun 21, 2025

SuGlider marked this pull request as draft July 15, 2025 13:48

SuGlider added 6 commits November 3, 2025 06:04

feat(uart): Refactor UART signal inversion handling

119082f

Refactor UART inversion functions to use a helper for signal inversion. Update UART bus array structure to include inversion mask.

feat(uart): Add UART signal inversion functions

268f282

Added functions for UART signal inversion and updated existing function signatures.

feat(uart): Refactor UART signal inversion handling

474edd3

feat(uart): Change setRxInvert and setTxInvert to return bool

4e94855

feat(uart) : Refactor serial inversion methods to return bool

2e3ae48

Changed setRxInvert, setTxInvert to return bool. Added setCtsInvert and setRtsInvert methods.

feat(uart): adds commentatries

af3ec5b

Added functions for UART pins signal inversion.

SuGlider reviewed Nov 3, 2025

View reviewed changes

cores/esp32/esp32-hal-uart.h Show resolved Hide resolved

SuGlider added 2 commits November 3, 2025 23:37

fix(uart): inverting rx instead of tx

06b2dfa

feat(uart): logging for UART signal inversion

c409993

Added logging for signal inversion in UART functions.

SuGlider reviewed Nov 4, 2025

View reviewed changes

cores/esp32/esp32-hal-uart.c Show resolved Hide resolved

SuGlider reviewed Nov 4, 2025

View reviewed changes

cores/esp32/esp32-hal-uart.c Outdated Show resolved Hide resolved

SuGlider added 2 commits November 3, 2025 23:57

feat(uart): standard verbose log message

c74f4e9

feat(uart): add not inverted verbose log message

ffe7769

SuGlider marked this pull request as ready for review November 4, 2025 03:02

SuGlider requested a review from Copilot November 4, 2025 03:02

SuGlider added Status: Review needed Issue or PR is awaiting review and removed Status: In Progress ⚠️ Issue is in progress labels Nov 4, 2025

SuGlider moved this from Under investigation to In Review in Arduino ESP32 Core Project Roadmap Nov 4, 2025

SuGlider modified the milestones: 3.2.1, 3.4.0 Nov 4, 2025

SuGlider requested a review from me-no-dev November 4, 2025 03:03

Copilot AI reviewed Nov 4, 2025

View reviewed changes

SuGlider self-requested a review November 4, 2025 03:07

SuGlider and others added 9 commits November 4, 2025 00:08

fix(uart): misspeling comment

6718c90

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(uart): fixes bad code formating

600e134

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(uart): fixes misspeling

1acfa42

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(uart): fixes bad code formating

949b366

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(uart): fixes bad code formating

f85e5f2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(uart): fixes bad code formating

bfad07c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(uart): fixes extra spaces

3992555

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(uart): extra spacing

3f6abf9

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

feat(uart): Improve logging for UART signal inversion

e6e09ce

SuGlider requested a review from Copilot November 4, 2025 03:14

Copilot AI reviewed Nov 4, 2025

View reviewed changes

SuGlider approved these changes Nov 4, 2025

View reviewed changes

me-no-dev approved these changes Nov 4, 2025

View reviewed changes

lucasssvaz approved these changes Nov 4, 2025

View reviewed changes

feat(uart): Add function to invert hardware UART Tx line #11428

Are you sure you want to change the base?

feat(uart): Add function to invert hardware UART Tx line #11428

Conversation

asund commented Jun 4, 2025 • edited by SuGlider Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Tests scenarios

Related links

Uh oh!

github-actions bot commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

SuGlider commented Jun 5, 2025

Uh oh!

asund commented Jun 5, 2025

Uh oh!

SuGlider commented Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asund commented Jun 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

asund commented Jun 4, 2025 •

edited by SuGlider

Loading

github-actions bot commented Jun 4, 2025 •

edited

Loading

github-actions bot commented Jun 4, 2025 •

edited

Loading

SuGlider commented Jun 21, 2025 •

edited

Loading